Skip to content

Add a solution#2683

Open
Slappy228 wants to merge 1 commit into
mate-academy:masterfrom
Slappy228:develop
Open

Add a solution#2683
Slappy228 wants to merge 1 commit into
mate-academy:masterfrom
Slappy228:develop

Conversation

@Slappy228

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am approving your solution because it meets all the functional requirements: it correctly reformats the date string based on fromFormat/toFormat, handles separator changes, supports reordering DD/MM/year, and implements YYYYYY conversion with the specified 20YY/19YY logic [CHECKLIST ITEMS #1#8]. The code also respects the constraint of assuming valid inputs without adding extra validation, which aligns with the task’s expectations [CHECKLIST ITEMS #9#11].

The main limitation is that the implementation is tightly coupled to the current example structure: it assumes exactly three date tokens and that the separator always lives at index 3 in both fromFormat and toFormat. This works for the described tests but would break if a future format moved the separator or changed the number of tokens; as an improvement, you could derive the separator and token positions dynamically by scanning for non-token entries rather than hardcoding indices and loop bounds. Another minor, non-blocking improvement would be to polish naming (e.g., splitedDatesplitDate) to improve readability.

Overall, your solution correctly fulfills the required behavior, and with the suggested adjustments you could make it more flexible and robust for future extensions.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment thread src/formatDate.js
*/
function formatDate(date, fromFormat, toFormat) {
// write code here
const splitedDate = date.split(fromFormat[3]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This split logic assumes the separator is always at fromFormat[3]. The description defines fromFormat as a format descriptor array but doesn’t guarantee that the separator is always at index 3; if that changes, this will fail. Consider finding the non-token element dynamically instead of hardcoding the index.

Comment thread src/formatDate.js

const objDate = {};

for (let i = 0; i < 3; i++) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop is hardcoded to exactly three components, which works for the provided examples, but it assumes the format will always have three date tokens. If fromFormat length or composition changes, this will not adapt; iterating based on the actual date-part entries in fromFormat would be more aligned with the description.

Comment thread src/formatDate.js

const result = [];

for (let item = 0; item < 3; item++) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, this loop assumes exactly three date-part positions in toFormat. While OK for the examples, it couples the logic to a specific array shape; using the actual date-part entries from toFormat instead of a magic number would make the function match the general “format descriptor” idea more closely.

Comment thread src/formatDate.js
}
}

return result.join(toFormat[3]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Joining with toFormat[3] implicitly assumes the separator is always at index 3 in toFormat. If the format arrays ever place the separator elsewhere, the output separator will be wrong. Deriving the separator by finding the non-YYYY/YY/MM/DD entry would avoid this assumption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants